RSDK-4089: use viam image #567

purplenicole730 · 2024-03-27T15:26:30Z

BREAKING CHANGE!!!!
Please note that this is going to be merged into `rc-0.21.0` to give time for the community to get used to the changes.

Changes:

return ViamImage when calling `camera.get_image()
use ViamImage for the vision service
remove SDK support for image processing
remove CameraMimeType functions and lazy concept
remove PIL.Image from the Python SDK (barring image examples and helper functions)
add a new media/utils file to contain helper functions
helper functions for converting ViamImage to PIL.Image and getting image dimensions

Testing:
Tested using a webcam and called both get_image_dimensions() as well as viam_to_pil_image().

github-actions · 2024-03-27T15:26:46Z

Warning your change may break code samples. If your change modifies any of the following functions please contact @viamrobotics/fleet-management. Thanks!

component	function
base	is_moving
board	gpio_pin_by_name
camera	get_image
encoder	get_position
motor	is_moving
sensor	get_readings
servo	get_position
arm	get_end_position
gantry	get_lengths
gripper	is_moving
movement_sensor	get_linear_acceleration
input_controller	get_controls
audio	get_properties
pose_tracker	get_poses
power_sensor	get_power
motion	get_pose
vision	get_classifications_from_camera

stuqdog

Nice, a couple small things but otherwise looks good to me!

I'm leaving this un-approved for now however because I believe (based on earlier conversation with @njooma) that we don't want to merge this into main but rather into a separate release branch 3 or 4 cycles from now, so that we give people plenty of time to prepare for the breaking change.

stuqdog · 2024-04-05T12:26:05Z

src/viam/media/utils.py

+        NotSupportedError: Raised if given an image that is not of MIME type `image/vnd.viam.dep`.
+    """
+    if image.mime_type not in [CameraMimeType.JPEG, CameraMimeType.PNG, CameraMimeType.VIAM_RGBA]:
+        NotSupportedError("Type must be `image/jpeg`, `image/png`, or `image/vnd.viam.rgba` to use get_image_dimensions()")


do we need to raise this error?

I wrote this whole paragraph why I thought having this error was necessary. And then realized you quite literally meant "hey, you forgot a raise keyword". 🤦

Thank you for reminding me I should be writing tests to make sure an error is raised if someone tries to call this function with the wrong mime type!

stuqdog · 2024-04-05T12:32:30Z

src/viam/media/utils.py

+    return depth_arr_2d
+
+
+def get_image_dimensions(image: ViamImage) -> None:


(minor) When I see a method named get_foo I typically expect that it will return a foo, I think it's a bit strange that this returns None. But it'd be weird to name it set_image_dimensions too because it's taking no image_dimension arguments! Can we rename to determine_image_dimensions or something like that?

Yes, this is a very good point!

njooma

Great work so far! This is looking good

njooma · 2024-04-11T14:47:28Z

src/viam/components/camera/client.py

+def get_image_from_response(data: bytes, response_mime_type: str, request_mime_type: str) -> ViamImage:
    if not request_mime_type:
        request_mime_type = response_mime_type
-    mime_type, is_lazy = CameraMimeType.from_lazy(request_mime_type)
-    if is_lazy or mime_type._should_be_raw:
-        image = RawImage(data=data, mime_type=response_mime_type)
-        return image
-    return Image.open(BytesIO(data), formats=LIBRARY_SUPPORTED_FORMATS)
+    mime_type = CameraMimeType.from_string(request_mime_type)
+    return ViamImage(data, mime_type)


This function is not needed anymore and is actually incorrect. We should just move this to the get_image function:

request = ... response: GetImageResponse = await self.client.GetImage(request, timeout=timeout) return ViamImage(response.image, response.mime_type)

njooma · 2024-04-11T15:52:15Z

src/viam/components/camera/service.py

+        if not request.mime_type:
+            if camera.name not in self._camera_mime_types:
+                self._camera_mime_types[camera.name] = CameraMimeType.JPEG
+
+            request.mime_type = self._camera_mime_types[camera.name]
+
+        response = GetImageResponse(mime_type=request.mime_type, image=image.data)


We actually don't need any of this anymore -- this was useful when we were encoding images. But because we're not doing that anymore, just sending raw bytes back, we should actually use the mime type returned from the camera (image.mime_type) instead of request.mime_type

njooma · 2024-04-11T15:56:06Z

src/viam/components/camera/service.py

-        finally:
-            image.close()
-        response = HttpBody(data=img, content_type=image.mime_type if isinstance(image, RawImage) else mimetype)  # type: ignore
+        response = HttpBody(data=image.data, content_type=image.mime_type if isinstance(image, ViamImage) else mimetype)  # type: ignore


This long check isn't needed anymore since image CAN ONLY ever be ViamImage. We can also remove the try/except block that attempts to get the mimetype.

njooma · 2024-04-11T15:57:47Z

src/viam/media/utils.py

+    Returns:
+        List[List[int]]: The standard representation of the image.
+    """
+    if image.mime_type != CameraMimeType.VIAM_RAW_DEPTH.value:


I don't think you need the .value at the end of CameraMimeType.VIAM_RAW_DEPTH.value (but I'm not 100% sure)

njooma · 2024-04-11T16:01:55Z

src/viam/media/video.py

@@ -159,10 +56,10 @@ class ViamImage:
    Provides the raw data and the mime type, as well as lazily loading and caching the PIL.Image representation.


This comment needs to be updated to remove the PIL reference

njooma · 2024-04-11T16:30:55Z

src/viam/media/video.py

+    def width(self) -> Union[int, None]:
+        """The width of the image"""
+        return self._width


Continuing from my earlier comment around keeping the _image and _image_decoded properties, the width and height getters should now be something like:

# First, check if we already have a value if self._width is not None: return self._width # If we don't have a value, check to see if we have already tried to decode the image # If we have not, then try to decode the image now if not self._image_decoded: try: self._image = Image.open(BytesIO(self.data), formats=LIBRARY_SUPPORTED_FORMATS) except UnidentifiedImageError: self._image = None self._image_decoded = True # If we have decoded the image and the image is not none, then set the width so we don't have to do this again if self._image_decoded and self._image is not None: self._width = self._image.width return self._width

njooma · 2024-04-11T16:34:13Z

src/viam/services/vision/vision.py

+            # Get image dimensions
+            determine_image_dimensions(img)
+


Remove this -- the user should never be calling this

njooma · 2024-04-11T16:34:29Z

src/viam/services/vision/vision.py

+            # Get image dimensions
+            determine_image_dimensions(img)
+


Same as above -- the user should never be calling this

njooma · 2024-04-11T16:35:50Z

tests/test_media.py

-        img.close()
+        assert img._mime_type == CameraMimeType.PNG
+        pil_img = viam_to_pil_image(img)
+        assert pil_img.tobytes() == i.tobytes()

    def test_mime_type_update(self):


We shouldn't allow users to update mime types anymore

tests/test_media.py

njooma

Minor update to the comments, but otherwise this is great

njooma · 2024-04-17T15:54:04Z

src/viam/components/camera/camera.py

        Be sure to close the image when finished.

-        NOTE: If the mime type is ``image/vnd.viam.dep`` you can use :func:`viam.media.video.ViamImage.bytes_to_depth_array`
+        NOTE: If the mime type is ``image/vnd.viam.dep`` you can use :func:`viam.media.utils.bytes_to_depth_array`


Revert this comment change, as bytes_to_depth_array is on ViamImage

purplenicole730 added 5 commits February 21, 2024 08:31

reintroduce last PR changes

94aa6ea

use viamimage in vision service

979929d

Merge branch 'main' into RSDK-4089-use-ViamImage

185a7a8

remove rawimage from camera files

485a851

use viamimage in camera_test

a0fd401

purplenicole730 added 23 commits March 27, 2024 08:28

fix imports in camera client test

414fae9

fix types and imports

34ebe4a

add height and width properties to viamimage

f7e355e

remove lazy concept from cameramimetype

3f5dfc2

start removing rawimage

2c98d45

add width and height to viamimage

2ce9dc6

change vision service test

bdb2713

remove encode_image

f885a41

save width and height in bytes_to_depth_array

27b669d

remove is_supported

66168f0

remove unsupported mime type

b217f71

update server example

9eb29f0

remove rawimage

58e4c2a

add and use helper function

7578283

move helper functions to new util file

6665030

test png image dimensions

bd747db

use pil image to get width and height

ac57979

add width and height tests

d75de48

add rgba test

94344ff

add jpeg test

a035aae

add clarifying comments

0609297

add more comments

010e0cf

raise viamerror instead of pil error

a8b9473

purplenicole730 marked this pull request as ready for review April 4, 2024 18:50

purplenicole730 requested a review from a team as a code owner April 4, 2024 18:50

purplenicole730 requested review from njooma and stuqdog April 4, 2024 18:50

stuqdog reviewed Apr 5, 2024

View reviewed changes

purplenicole730 added 2 commits April 5, 2024 16:18

rename image dimension function

75c1698

test not supported calls to util functions

0b1f38a

purplenicole730 changed the base branch from main to rc-0.21.0 April 11, 2024 13:50

njooma requested changes Apr 11, 2024

View reviewed changes

purplenicole730 added 3 commits April 15, 2024 13:09

add pr feedback

c894f8e

determine dimensions internally

71c2a1c

rename pil utils file to pil.py

2f6c5fa

purplenicole730 requested a review from njooma April 16, 2024 20:31

purplenicole730 added 2 commits April 17, 2024 11:27

re-add comment

83f8444

move pil to utils

a05f6c6

njooma approved these changes Apr 17, 2024

View reviewed changes

purplenicole730 added 2 commits April 17, 2024 12:14

revert comment

580b67f

undo change

96f1fcb

purplenicole730 merged commit 68dd7f9 into viamrobotics:rc-0.21.0 Apr 17, 2024
11 checks passed

purplenicole730 deleted the RSDK-4089-use-ViamImage branch April 17, 2024 16:44

njooma pushed a commit that referenced this pull request Apr 18, 2024

RSDK-4089: use viam image (#567)

b63352c

njooma pushed a commit to njooma/viam-python-sdk that referenced this pull request Apr 18, 2024

RSDK-4089: use viam image (viamrobotics#567)

28a7213

njooma pushed a commit to njooma/viam-python-sdk that referenced this pull request Apr 30, 2024

RSDK-4089: use viam image (viamrobotics#567)

ac1803a

njooma pushed a commit that referenced this pull request May 14, 2024

RSDK-4089: use viam image (#567)

a1cf089

njooma pushed a commit that referenced this pull request May 20, 2024

RSDK-4089: use viam image (#567)

a21e5ec

hexbabe mentioned this pull request Jul 17, 2024

[RSDK-6312] Change OAK module method return types to ViamImage viam-modules/viam-camera-oak#47

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

RSDK-4089: use viam image #567

RSDK-4089: use viam image #567

purplenicole730 commented Mar 27, 2024 •

edited

Loading

github-actions bot commented Mar 27, 2024

stuqdog left a comment

stuqdog Apr 5, 2024

purplenicole730 Apr 5, 2024

purplenicole730 Apr 5, 2024

stuqdog Apr 5, 2024

purplenicole730 Apr 5, 2024

njooma left a comment

njooma Apr 11, 2024

njooma Apr 11, 2024

njooma Apr 11, 2024

njooma Apr 11, 2024

njooma Apr 11, 2024

njooma Apr 11, 2024

njooma Apr 11, 2024

njooma Apr 11, 2024

njooma Apr 11, 2024

njooma left a comment

njooma Apr 17, 2024

		return depth_arr_2d


		def get_image_dimensions(image: ViamImage) -> None:

		@@ -159,10 +56,10 @@ class ViamImage:
		Provides the raw data and the mime type, as well as lazily loading and caching the PIL.Image representation.

RSDK-4089: use viam image #567

RSDK-4089: use viam image #567

Conversation

purplenicole730 commented Mar 27, 2024 • edited Loading

BREAKING CHANGE!!!! Please note that this is going to be merged into rc-0.21.0 to give time for the community to get used to the changes.

github-actions bot commented Mar 27, 2024

stuqdog left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

njooma left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

njooma left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

purplenicole730 commented Mar 27, 2024 •

edited

Loading

BREAKING CHANGE!!!!
Please note that this is going to be merged into `rc-0.21.0` to give time for the community to get used to the changes.